feat: Add Sana styles to Canvas Switch, Radio & Checkbox#4017
feat: Add Sana styles to Canvas Switch, Radio & Checkbox#4017RayRedGoose wants to merge 20 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughRadio, Switch, and Checkbox stencils update token sources, sizes, colors, and selected state styles. Related visual testing stories add ChangesRadio, Switch, and Checkbox styling
Story updates
Upgrade guide documentation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~28 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Adds Sana-aligned styling updates for the Switch, Radio, and Checkbox components to reflect updated token usage and visual specs (Issue #3984).
Changes:
- Updated sizing tokens across Checkbox/Radio/Switch (moving several values from
base.legacy.*tosystem.legacy.size.*) and adjusted spacing/offsets accordingly. - Updated accent colors (notably moving checked states toward
brand.accent.positive) and refined hover/focus/error ring visuals. - Updated Checkbox label rendering/styling (LabelText → Subtext) and introduced part-based styling in the container stencil.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| modules/react/checkbox/lib/CheckboxRipple.tsx | Updates ripple sizing to new system.legacy.size.* tokens. |
| modules/react/checkbox/lib/CheckboxInput.tsx | Adjusts checkbox dimensions, hover ripple size, checked colors, focus/error ring visuals. |
| modules/react/checkbox/lib/CheckboxContainer.tsx | Updates label component/styling and adds stencil parts/modifiers for Sana spacing/disabled styling. |
| modules/react/checkbox/lib/CheckboxCheck.tsx | Tunes checkmark sizing/alignment and inverse variant colors. |
| modules/react/checkbox/lib/CheckBackground.tsx | Updates checkbox background shape/tokens and error styling variables. |
| modules/preview-react/switch/lib/SwitchInput.tsx | Updates input height token to align with new spec sizing. |
| modules/preview-react/switch/lib/SwitchIcon.tsx | Tweaks icon positioning to match updated switch geometry. |
| modules/preview-react/switch/lib/SwitchContainer.tsx | Updates container height token for Sana sizing. |
| modules/preview-react/switch/lib/SwitchCircle.tsx | Updates thumb sizing and checked translation distances. |
| modules/preview-react/switch/lib/SwitchBackground.tsx | Updates background height, padding, and muted accent background token. |
| modules/preview-react/radio/lib/StyledRadioButton.tsx | Updates radio sizing tokens, checked colors, and hover ripple sizing. |
| modules/preview-react/radio/lib/RadioLabel.tsx | Updates label min-height token. |
| modules/preview-react/radio/lib/RadioGroup.tsx | Adjusts group border radius/padding and simplifies error backgrounds. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Workday/canvas-kit
|
||||||||||||||||||||||||||||||||||||||||
| Project |
Workday/canvas-kit
|
| Branch Review |
issue3984-checkbox-radio
|
| Run status |
|
| Run duration | 02m 27s |
| Commit |
|
| Committer | Raisa Primerova |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
|
|
0
|
|
|
0
|
|
|
17
|
|
|
0
|
|
|
809
|
| View all changes introduced in this branch ↗︎ | |
UI Coverage
19.7%
|
|
|---|---|
|
|
1522
|
|
|
371
|
Accessibility
99.37%
|
|
|---|---|
|
|
5 critical
5 serious
0 moderate
2 minor
|
|
|
72
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@modules/react/checkbox/lib/CheckboxContainer.tsx`:
- Around line 37-48: The disabled modifier in the modifiers object only
overrides the cursor for the container, but the labelPart still has cursor set
to 'pointer' from its base styles, causing the pointer cursor to remain visible
when the checkbox is disabled. Add a disabled true override within the modifiers
section for the labelPart that sets its cursor to 'default' to match the
disabled state behavior of the container.
In `@modules/react/checkbox/lib/CheckboxInput.tsx`:
- Around line 178-179: The error-ring box-shadow at lines 178-179 uses the same
px2rem(1) spread radius for both the errorRingColorOuter and errorRingColorInner
layers, causing the inner ring to paint over and hide the outer ring. Update the
spread radius values for each layer to use distinct values that preserve the
two-layer visual treatment. The first layer (errorRingColorOuter) and second
layer (errorRingColorInner) should each have different px2rem values to ensure
both rings are visible in the unchecked error state.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: d2e70c34-8bcc-40ec-b167-64a486cb0022
📒 Files selected for processing (13)
modules/preview-react/radio/lib/RadioGroup.tsxmodules/preview-react/radio/lib/RadioLabel.tsxmodules/preview-react/radio/lib/StyledRadioButton.tsxmodules/preview-react/switch/lib/SwitchBackground.tsxmodules/preview-react/switch/lib/SwitchCircle.tsxmodules/preview-react/switch/lib/SwitchContainer.tsxmodules/preview-react/switch/lib/SwitchIcon.tsxmodules/preview-react/switch/lib/SwitchInput.tsxmodules/react/checkbox/lib/CheckBackground.tsxmodules/react/checkbox/lib/CheckboxCheck.tsxmodules/react/checkbox/lib/CheckboxContainer.tsxmodules/react/checkbox/lib/CheckboxInput.tsxmodules/react/checkbox/lib/CheckboxRipple.tsx
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@modules/docs/mdx/16.0-UPGRADE-GUIDE.mdx`:
- Line 145: Replace the word "team" with "theme" in the phrases that mention
"`sana-canvas` team" in the UPGRADE-GUIDE.mdx file. This typo appears in three
locations where it should read "`sana-canvas` theme" instead. Search for all
instances of "`sana-canvas` team" and change them to "`sana-canvas` theme" to
correct the terminology.
- Around line 97-119: Replace all instances of the attribute name
`data-attribute="sana-canvas"` with `data-theme="sana-canvas"` throughout this
upgrade guide section. The correct attribute name that the runtime wiring uses
is `data-theme`, not `data-attribute`. Update this in all code examples showing
both global application (in the body tag) and local scoped application (in the
div tag) to ensure users enable the Sana Canvas theme correctly.
- Around line 140-141: The migration guide contains incorrect token namespace
paths that are missing the `.legacy` prefix. Update all references to
`system.color.brand.accent.positive` and `system.color.brand.accent.primary` to
use `system.legacy.color.brand.accent.positive` and
`system.legacy.color.brand.accent.primary` respectively. This change is needed
in three locations: the Checkbox section (around lines 140-141), the Radio
section (around lines 154-155), and the Switch section (around lines 168-171).
Add the `.legacy` prefix between `system.color` and `.brand.accent` in each
occurrence to align the documentation with the actual component implementation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: d8303dc8-fe87-4074-99f4-88a3b1bf9723
📒 Files selected for processing (1)
modules/docs/mdx/16.0-UPGRADE-GUIDE.mdx
There was a problem hiding this comment.
🧹 Nitpick comments (1)
modules/docs/mdx/16.0-UPGRADE-GUIDE.mdx (1)
131-174: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd migration guidance for
inversedeprecation in Radio/Checkbox sections.These component update blocks list visual changes, but they should also call out the
inversevariant deprecation and codemod path so adopters don’t miss required migration work in v16.Suggested doc addition
### Checkbox **PR:** [`#3984`](https://github.com/Workday/canvas-kit/pull/3984) #### Visual Updates @@ - The hover ring is smaller to better align with the smaller size. The hover ring background color uses neutral instead of slate in `sana-canvas` theme. + +#### Deprecations + +- `inverse` is deprecated in v16. Use the standard Sana theming approach with + `data-theme="sana-canvas"` instead. +- 🤖 Codemod support: run the v16 codemod (`@workday/canvas-kit-codemod`) to apply supported + automatic migrations. @@ ### Radio @@ #### Visual Updates @@ - The hover ring is smaller to better align with the smaller size. The hover ring background color uses a neutral color instead of slate in `sana-canvas` theme. + +#### Deprecations + +- `inverse` is deprecated in v16. Use the standard Sana theming approach with + `data-theme="sana-canvas"` instead. +- 🤖 Codemod support: run the v16 codemod (`@workday/canvas-kit-codemod`) to apply supported + automatic migrations.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@modules/docs/mdx/16.0-UPGRADE-GUIDE.mdx` around lines 131 - 174, The Checkbox and Radio component update sections currently list only visual changes but omit important migration guidance for the deprecated `inverse` variant. Add a subsection under both the Checkbox and Radio sections that explicitly documents the deprecation of the `inverse` variant and provides the codemod path or migration instructions so adopters are aware of the required changes needed for v16 upgrade.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@modules/docs/mdx/16.0-UPGRADE-GUIDE.mdx`:
- Around line 131-174: The Checkbox and Radio component update sections
currently list only visual changes but omit important migration guidance for the
deprecated `inverse` variant. Add a subsection under both the Checkbox and Radio
sections that explicitly documents the deprecation of the `inverse` variant and
provides the codemod path or migration instructions so adopters are aware of the
required changes needed for v16 upgrade.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: f27f2c83-b298-44b3-aef4-df9adea8e543
📒 Files selected for processing (2)
modules/docs/mdx/16.0-UPGRADE-GUIDE.mdxmodules/react/checkbox/lib/CheckboxContainer.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- modules/react/checkbox/lib/CheckboxContainer.tsx
sheelah
left a comment
There was a problem hiding this comment.
LG - left just a few minor comments.
| @@ -78,8 +84,8 @@ const checkboxInputStencil = createStencil({ | |||
|
|
|||
| [`&:where(:checked, :indeterminate) ~ [data-part="${checkboxBackgroundStencil.parts.background['data-part']}"]`]: | |||
There was a problem hiding this comment.
Medium: checked-state contrast now depends on success.main. Checkbox, radio, and switch checked states now use system.legacy.color.brand.accent.positive, while the icon/thumb foreground remains inverse/white. That is fine for dark success colors, but consumer themes may have light success colors. The visual test theme changing success.main to darkolivegreen helps the test pass, but does not protect consumers. Consider using a paired contrast token for the foreground, documenting a minimum contrast requirement for success.main, or adding themed contrast coverage with a light success color
There was a problem hiding this comment.
this is something we cannot actually control and predict what color is used to theme, this possibility can happen also for accent.primary if someone choose lite colors for their brand. I think it's worth to add docs around that.
| ### Accessibility: Checked-State Contrast | ||
|
|
||
| > **Important:** The checked states of `Checkbox`, `Radio`, and `Switch` now derive their | ||
| > background/fill color from `system.color.brand.accent.positive` (which maps to your theme's | ||
| > `brand.success.600`), while the foreground (the check icon, radio dot, and switch thumb) remains | ||
| > an inverse/white color. This pairing relies on `brand.success.600` being a sufficiently dark | ||
| > color. |
There was a problem hiding this comment.
@williamjstanton I've added this section to upgrade guide to highlight changes to success color, as we have to use that colors and cannot control how it's used.
Summary
Fixes: #3984
Added Sana styles to Canvas Switch, Radio & Checkbox
Release Category
Components
Checklist
ready for reviewhas been added to PRFor the Reviewer
Where Should the Reviewer Start?
Areas for Feedback? (optional)
Testing Manually
Screenshots or GIFs (if applicable)
Thank You Gif (optional)
Summary by CodeRabbit
Release Notes
Refactor
Style
Documentation
Tests
data-theme="sana-canvas".